The sidebar stops spending columns on nothing, and a background tab says whose it is - #33
Conversation
The connection rail was 22 columns wide in the demo's `connections` frame while its widest visible row ended at 20 and most ended at 15. Two of those columns named nothing. The indent ladder skipped a level: worlds at 0, characters straight to 2, windows at 3, so every window row carried two cells of indent for a depth no row is ever drawn at. It is one level per depth now — world 0, character 1, window 2. `RailMargin` goes 2 → 1. A divider column and a one-cell spacer already stand between the rail's last cell and the first pane, so the second margin cell separated nothing from nothing. The rail's width comes out of the pane area and every connected session is told its pane's size over NAWS, so both are columns handed back to the game. Demo `connections` rail: 22 → 19. The reserved fields are untouched — the unsent pen, the unread badge and the chord column still cost their cells whether or not they have anything to say, because a field that appears out of nothing resizes the sidebar from the wire. Two pins: `TheIndentLadderSkipsNoLevel` (no row is more than one level deeper than the row before it) and `TheRailSpendsAtMostOneBlankColumnPast ItsWidestRow`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN
A pane can host several characters' windows as tabs and paints one
rectangle, so the plane behind a strip can only ever answer for the window
in front. Every other tab in that strip was drawn in that same hue —
naming the wrong character, which is the one thing the pane tint exists to
prevent.
`TabControl`'s four chip colours belong to the control, so this cannot come
from `PaintTabChips`. It goes where the per-tab channel is: the title, which
is markup, where a tag costs no cells and moves no hit test. `TabChip`
carries the plane and its ink, and `TabTitles.For` emits `[{ink} on {plane}]`
around the same span the bold and the activity tint already cover.
`ChipFor` runs the same pipeline as the plane behind it — `SurfaceToneIn`
(the character's tint, plus the focus lift when the pane holds focus) then
`Recessed` — so a strip whose tabs share an owner comes out byte for byte
what it was, and only a mixed pane changes. `PaneSurfaceTone` is now one
line of that function rather than a second arithmetic beside it. The
selected tab is never chipped: the strip already paints it in its page's
plane. Unread stays a foreground over the chip, so "whose" and "something
new" remain two channels. A window nobody owns and a character who has
chosen no colour come out on the plain surface.
New `tint-tabs` view: the two tinted characters of `tint` with no split, so
one pane holds both their windows. It is the only geometry where an idle
chip can be seen wearing a colour the pane behind it is not.
`PaneTintTests.ABackgroundTabWearsItsOwnCharactersColour` reads the painted
cells — the old code was internally consistent while the screen was wrong,
so nothing built on the expression would have caught it. `TabTitlesTests`
pins the markup's shape and that a chip costs no cells.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe change adds per-character tinting for idle tab chips, preserves selected and unread tab behavior, and adds ChangesPer-character tab chip tinting
Rail indentation and spacing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SharpMUTermApp
participant TabTitles
participant TerminalFrame
SharpMUTermApp->>SharpMUTermApp: derive TabChip from window tint and pane focus
SharpMUTermApp->>TabTitles: build or refresh tab title with TabChip
TabTitles->>TerminalFrame: render per-character idle tab styling
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/SharpMUTerm.Core.Tests/Workspace/RailModelTests.cs`:
- Around line 212-233: Strengthen TheIndentLadderSkipsNoLevel by asserting each
row’s exact indent based on its Kind: worlds at 0, character and empty rows at
1, and windows at 2. Keep the existing ordering and maximum-indent checks, and
apply the assertions while iterating over rows so incorrect empty-row
indentation is detected.
In `@tests/SharpMUTerm.Tui.Tests/PaneTintTests.cs`:
- Around line 135-175: Add a companion frame test near
ABackgroundTabWearsItsOwnCharactersColour that renders a single pane containing
a tinted window and an unowned window such as the web view, with the unowned
window backgrounded. Locate both tab labels via FrameGrid.Decode and assert the
unowned chip’s background is
WorkspacePalette.Recessed(WorkspacePalette.Focus(WorkspacePalette.Tint(config.Theme,
PaneTint.None))) while the tinted chip uses its own tint, confirming the colours
differ.
In `@tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs`:
- Around line 244-262: Update TheRailSpendsAtMostOneBlankColumnPastItsWidestRow
to assert rendered geometry rather than only comparing RailColumnWidth with the
widest logical rail row. After RenderSnapshot, inspect the arranged rail bounds
and the adjacent pane rectangle (or NAWS-derived width), and verify the
one-column margin relationship using those actual layout values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6957108e-8949-4423-bf61-a463866db137
📒 Files selected for processing (9)
CLAUDE.mdsrc/SharpMUTerm.Core/Workspaces/RailModel.cssrc/SharpMUTerm.Tui/SharpMUTermApp.cssrc/SharpMUTerm.Tui/TabTitles.cstests/SharpMUTerm.Core.Tests/Workspace/RailModelTests.cstests/SharpMUTerm.Tui.Tests/PaneTintTests.cstests/SharpMUTerm.Tui.Tests/RailRendererTests.cstests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cstests/SharpMUTerm.Tui.Tests/TabTitlesTests.cs
All three were the same shape — a test that agreed with the code it was watching rather than with the screen or the model. `TheIndentLadderSkipsNoLevel` proved the ladder was monotonic and capped at 2, which an Empty row at indent 0 satisfies. Each kind is now held to its own depth. `TheRailSpendsAtMostOneBlankColumnPastItsWidestRow` was arithmetic over the rail's own rows and would have passed had nothing applied the answer. It closes against the arranged pane rectangle now: a pane starts past the rail, its divider and the spacer — the same geometry per-pane NAWS is derived from. `AnUnownedBackgroundTabBorrowsNobodysColour` is new: the other half of the chip rule, that a tab nobody owns stays on the plain surface rather than borrowing the hue of whatever is in front of it. The web view is the reachable case and the one that would make the cue name a character whose window it is not. It asserts the pane really is tinted too, or the frame asks nothing; confirmed it fails against the old per-pane chip with `expected Rgb(41,41,46), found Rgb(17,34,71)`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN
Two things reported while reading a frame, fixed separately and reviewable apart (one commit each).
1. The connection rail was too wide for what it holds
c9aa311. The demo'sconnectionsframe drew a 22-column sidebar whose widest visible row ended at column 20 and whose typical row ended at 15. Two of those columns named nothing:RailMarginwas 2. A divider column and a one-cell spacer already stand between the rail's last cell and the first pane, so the second margin cell separated nothing from nothing. It is 1.The rail's width comes out of the pane area and every connected session is told its pane's size over NAWS, so both are columns handed back to the game. Demo
connections: 22 → 19.The reserved fields are untouched — the unsent pen, the unread badge and the chord column still cost their cells when empty, because a field that appears out of nothing resizes the sidebar from the wire.
Pins:
RailModelTests.TheIndentLadderSkipsNoLevel(no row is more than one level deeper than the row before it) andRailWindowRowTests.TheRailSpendsAtMostOneBlankColumnPastItsWidestRow. One existing assertion had the old 4-cell indent baked into a string literal.2. A background tab wore the wrong character's colour
6a32756. A pane can host several characters' windows as tabs and paints one rectangle, so the plane behind a strip can only ever answer for the window in front. Every other tab in that strip was drawn in that same hue — naming the wrong character, which is the one thing the pane tint exists to prevent.TabControl's four chip colours belong to the control, so this cannot come fromPaintTabChips. It goes where the per-tab channel is: the title, which is markup, where a tag costs no cells and moves no hit test.TabChipcarries the plane and its ink;TabTitles.Foremits[{ink} on {plane}]around the same span the bold and the activity tint already cover.Three things hold it together:
ChipForruns the same pipeline as the plane behind it —SurfaceToneIn(the character's tint, plus the focus lift when the pane holds focus) thenRecessed— so a strip whose tabs share an owner comes out byte for byte what it was, and only a mixed pane changes.PaneSurfaceToneis now one line of that same function rather than a second arithmetic beside it.A window nobody owns (the web view) and a character who has chosen no colour both come out on the plain surface rather than borrowing a neighbour's hue.
New
tint-tabssnapshot view: the two tinted characters oftintwith no split, so one pane holds both their windows. It is the only geometry where an idle chip can be seen wearing a colour the pane behind it is not. The frame readsThistle ⌁ │ Corvid - Chat (2) │ ▌ CorvidwithThistleon Moss,Chaton recessed Slate and the selectedCorvidon lit Slate.PaneTintTests.ABackgroundTabWearsItsOwnCharactersColourreads the painted cells: the old code was internally consistent while the screen was wrong, so anything built on the expression would have passed. Confirmed it discriminates by temporarily resolving the pane's front window instead — it failed withexpected Rgb(20,42,14), found Rgb(17,34,71).TabTitlesTestspins the markup's shape and that a chip costs no cells.Verification
dotnet build SharpMUTerm.slnxwarning-free, and all five suites green: Core 937, Graphics 83, Scripting 42, Web 37, Tui 1777.🤖 Generated with Claude Code
https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN
Summary by CodeRabbit
New Features
Bug Fixes